Skip to content

Annotation conversion guidance for SSL and gRPC backend (upstream) services #886

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Annotation conversion guidance for SSL and gRPC backend (upstream) services #886

wants to merge 4 commits into from
+49 −0

Conversation

jnewfield
Copy link
Contributor

@jnewfield jnewfield commented Jul 23, 2025 •
edited by ADubhlaoich
Loading

Proposed changes

Problem: Add annotation conversion guidance for SSL and gRPC backend (upstream) services from Kubernetes Ingress-NGINX Controller to F5 NGINX Ingress Controller

Solution: Document conversions

Testing: I implemented F5 NGINX Ingress Controller annotations that are being documented in lab NIC environment to confirm they function as expected

Checklist

Before merging a pull request, run through this checklist and mark each as complete.

  • I have read the contributing guidelines
  • I have signed the F5 Contributor License Agreement (CLA)
  • I have rebased my branch onto main
  • I have ensured my PR is targeting the main branch and pulling from my branch from my own fork
  • I have ensured that the commit messages adhere to Conventional Commits
  • I have ensured that documentation content adheres to the style guide
  • If the change involves potentially sensitive changes1, I have assessed the possible impact
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • I have ensured that existing tests pass after adding my changes
  • If applicable, I have updated README.md

Footnotes

  1. Potentially sensitive changes include anything involving code, personally identify information (PII), live URLs or significant amounts of new or revised documentation. Please refer to our style guide for guidance about placeholder content. ↩

@jnewfield jnewfield requested a review from a team as a code owner July 23, 2025 22:15
@github-actions github-actions bot added documentation Improvements or additions to documentation product/nic NGINX Ingress Controller related issues labels Jul 23, 2025
@ADubhlaoich ADubhlaoich requested a review from a team July 24, 2025 12:10
ADubhlaoich
ADubhlaoich requested changes Jul 24, 2025
View reviewed changes
Copy link
Contributor

@ADubhlaoich ADubhlaoich left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed the internal resource link from the PR description.

Could you reformat these snippets into a table?

There are some other small style conventions I can address as suggestions afterwards.

I would also like a different PR title, as the current one had no indication of what the context of the PR would be.

You might find our Commit messages guidelines useful.

@ADubhlaoich
Copy link
Contributor

On further investigation, I'm curious what the intended use case for this specific detail is.

We have a Migrate from Ingress-NGINX Controller document that already include a table mapping the Ingress-NGINX resources: is there a reason these could not be added to the existing table?

@jnewfield jnewfield changed the title Changes have been made Annotation conversion guidance for SSL and gRPC backend (upstream) services Jul 24, 2025
@jnewfield
Copy link
Contributor Author

jnewfield commented Jul 24, 2025 •
edited
Loading

I thought about putting these annotations in the existing table. However, the proposed annotation conversions are predicated on corresponding values. All of the mapping tables in the document contain just annotations without values. Configuration snippets contain annotations and values. For example, implementing just nginx.ingress.kubernetes.io/backend-protocol and nginx.org/ssl-services would not provide guidance for a successful conversion. However, nginx.ingress.kubernetes.io/backend-protocol: "HTTPS" to nginx.org/ssl-services: "ssl_service_name" does.

@jnewfield
Copy link
Contributor Author

@ADubhlaoich
What are your thoughts on my last comment?

@ADubhlaoich
Copy link
Contributor

ADubhlaoich commented Aug 11, 2025 •
edited
Loading

Hi @jnewfield, my apologies for not following up on this PR. Five day weekend and short week meant I missed a few things to follow up on.

I guess it makes sense to keep them separate, though I may combine them together at a later date.

If the content is converted to a table, I'll be happy to approve. I'll also bump this up to the engineering team, since none of them have weighted in yet.

Stylistically, we avoid using horizontal lines nowadays (---) and the note should follow contemporary call-out guidance: Add call-outs to documentation page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ADubhlaoich ADubhlaoich ADubhlaoich requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation product/nic NGINX Ingress Controller related issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jnewfield @ADubhlaoich